fix: normalize locked orchestrator health response bodies - #695
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesHealth 응답 스트림 오류 처리
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change is narrowly scoped and preserves existing fallback behavior; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review on c9d526093e543167645383e937096fd089b337f8: the diff is limited to normalizing synchronous health-response reader acquisition failure plus its hostile locked-stream regression. Existing byte ceiling, timeout semantics, bounded retained buffer, cancellation liveness, UTF-8/duplicate-key JSON admission, and contextual-orchestrator ownership boundary remain unchanged. Application CI, reviewer-ci, required Security Scan, and patch-validator-image are all terminal SUCCESS; CodeRabbit reports no actionable finding; unresolved inline threads: 0. COMMENT only; this is not self-approval.
…alth-reader-authority-20260913 docs: converge protected #695 health reader authority
Problem
The contextual-orchestrator
/healthzverifier already bounds streamed response bytes and makes cancellation best-effort, but it still calledresponse.body.getReader()without normalizing synchronous reader-acquisition failure. A legitimately locked/non-stream-readableReadableStreamcould therefore surface implementation-specific stream errors instead of one stable Noema transport diagnostic.This is a Noema-owned health-response consumption/error-admission repair only. It does not move contextual-orchestrator service/provider/model routing, credentials, outbound authority, quarantine/security authority, release/deployment authority, or foreign domain truth into Noema.
Test-first RED
Test-only exact head:
b66ee03c5a0ce57c7e346162353e4e7b852de0a0.The hostile regression holds a real
Response.bodyreader beforeverifyOrchestratorHealthz()attempts acquisition and requires the stable diagnostic:contextual-orchestrator health response body is not stream-readableA direct runtime reproduction against the protected implementation produced the underlying failure (
TypeError: Invalid state: ReadableStream is locked), proving the current acquisition path leaked stream implementation detail. Hosted Actions for the test-only head entered the repository runner queue and had not been assigned when the minimal source repair was committed; the test-only commit remains immutable evidence.Minimal repair
Current exact head:
c9d526093e543167645383e937096fd089b337f8.Only health-response reader acquisition is wrapped. Synchronous acquisition failure is normalized to the stable transport diagnostic. The existing 65,536-byte ceiling, optional caller-supplied deadline (no invented default timeout), fixed retained buffer, best-effort cancellation, fatal UTF-8, duplicate-key/JSON admission,
orchestrator/freerouting contract, and contextual-orchestrator ownership boundary are unchanged.Verification
Final exact-head hosted CI/review/security/image gates are required before merge. No gate weakening, provider fallback, direct provider routing, self-approval, force-push, or destructive rebase is permitted.
Summary by CodeRabbit